Refactor SlippiNetplayClient to enable attempting connections to multiple endpoints per player #389
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…e applicable tested singles singles via LAN singles vs symmetric NAT doubles vs double symmetric NAT
this is necessary because early disconnects are possible (in case of network failures, physical disconnect, etc)
|
Oh boy. Yeah this changes a lot of critical things. Some of the stuff that got changed were things that I added because there were issues with people disconnecting from each other 30 seconds into a match, for example. Would be good to do some decent testing and also have a decent revert plan if something breaks. |
|
I'll have to think more about a testing plan. The main problem I'm thinking of is that this PR alone doesn't offer any improved functionality to entice testers (in 99.9% of cases). In the meantime can you tell me about the 30 second disconnect? What parts of the original code address that? I can at least try to make sure there's no regressions. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(at this time, the only possible endpoints are external and local)
Because this rewrites much of the connection logic, I tested this in the following cases
In the immediate term this will fix connections for the case where
Before #387 is merged this will also fix connections for the case where
In the future there is the possibility of using this functionality to